-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make testing easier by adding a disable() method #40
Conversation
Moving the |
To be honest I think at this point it would make sense to just get rid of the |
Also I think I would prefer to see something like |
Something like this ? I also like |
@@ -49,8 +49,14 @@ public function boot() | |||
$translator = $app['translator']; | |||
|
|||
// Add honeypot and honeytime custom validation rules | |||
$validator->extend('honeypot', 'Msurguy\Honeypot\HoneypotValidator@validateHoneypot', $translator->get('honeypot::validation.honeypot')); | |||
$validator->extend('honeytime', 'Msurguy\Honeypot\HoneypotValidator@validateHoneytime', $translator->get('honeypot::validation.honeytime')); | |||
$validator->extend('honeypot', function($attribute, $value, $parameters) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't need anonymous functions anymore. Just Msurguy\Honeypot\Honeypot@validateHoneypot
Looks good to me. Tests pass? |
@@ -69,6 +69,28 @@ Please note that "honeytime" takes a parameter specifying number of seconds it s | |||
|
|||
That's it! Enjoy getting less spam in your inbox. If you need stronger spam protection, consider using [Akismet](https://github.com/kenmoini/akismet) or [reCaptcha](https://github.com/dontspamagain/recaptcha) | |||
|
|||
## Testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs updating
Once updated I would suggest to squash the commits. |
The problem with using Class@Method instead of a closure is that is creates a new instance to validate, so disable() doesn't work. |
Gotcha. I'm pretty sure the validator resolves the class thru the IOC so in theory if you did |
Indeed, seems to work. |
Tests didn't pass after I cloned:
|
It should be |
…isable the validation
Documentation updated and commits squashed. |
Looks good. 👍 |
Make testing easier by adding a disable() method
This PR is related to issue #39 and is a suggestion to allow easy Validator mocking:
I have updated the documentation.